NXP backend: Add quantization of aten.view #11784
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/11784
Note: Links to docs will display an error until the docs builds have been completed. ❌ 3 Cancelled JobsAs of commit cf1aa3c with merge base 75e4044 ( CANCELLED JOBS - The following jobs were cancelled. Please retry:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
facebook-github-bot
added
the
CLA Signed
robert-kalmar
added
module: nxp
|
nit: can you split this diff? It seems to be two completely separate changes, aten view quantization and the removal of avg pool/linear optimization |
StrycekSimon
force-pushed
the
upstream/main-nxp/EIEX-300-upstream-aten.view-support
branch
from
c30ba16 to
f5c779e
Compare
StrycekSimon
force-pushed
the
upstream/main-nxp/EIEX-300-upstream-aten.view-support
branch
from
f5c779e to
cf1aa3c
Compare
|
LGTM |
| @@ -164,9 +160,6 @@ def __init__( | |||
| Optimization.COMBINE_HARD_SIGMOID_AND_MUL_INTO_HARD_SWISH: CombineHardSigmoidAndMulIntoHardSwish( | |||
| builder, conversion_config | |||
| ), | |||
| Optimization.REPLACE_AVERAGE_POOL_BEFORE_FULLY_CONNECTED_WITH_SUM: ReplaceAveragePoolBeforeFullyConnectedWithSum( | |||
There was a problem hiding this comment.
How is this related to the view support?
There was a problem hiding this comment.
Same here, not related to aten.view. Our fault.
It is indeed a separate commit, and the PR descrition should have included the proper description:
"Fix avegage pooling, which in case of kernel (1,1) turns into Sum in Neutron IR"
At least added in to the PR description.
| @@ -47,37 +39,6 @@ def test_constant_pad_nd_conversion__specific_constant(constant): | |||
| convert_run_compare(edge_program, input_data) | |||
|
|
|||
|
|
|||
| @pytest.mark.parametrize("constant", [0.0, 67.28, 42.0, -13.37]) | |||
| @pytest.mark.skip(reason="Neutron Converter does not fully convert for NPU") | |||
| def test_constant_pad_nd_quant_conversion__specific_constant(mocker, constant): | |||
There was a problem hiding this comment.
You are right, this should have been a separate PR to remove irrelevant test. We will pay more attention when rebasing from our development tree.
Summary
Updates part of this PR:
Test plan
Unit test part of the change.
cc @digantdesai @JakeStevens